Skip to content

fix: use task workspace for codebase search - #4

Open
WebMad wants to merge 22 commits into
refactor/1594-code-index-manager-registry-incrementalfrom
fix/codebase-search-task-workspace
Open

WebMad wants to merge 22 commits into
refactor/1594-code-index-manager-registry-incrementalfrom
fix/codebase-search-task-workspace

Conversation

@WebMad

@WebMad WebMad commented Sep 13, 2026

Copy link
Copy Markdown
Owner

Summary

  • Pass the already resolved task workspace to the code index manager registry instead of allowing active-editor workspace selection.
  • Keep the existing fallback for empty/whitespace task working directories.
  • Add regression coverage using the real registry and mocked managers, verifying root B is searched while the active editor belongs to root A, query/path forwarding, result output, fallback selection, and missing workspace handling.

Stacked PR

Depends on Zoo-Code-Org#1622.
Branched from its exact head: 8637e48.
This PR targets the parent branch in WebMad/Zoo-Code so the diff contains only this bugfix and its tests, not the parent refactor. After the parent merges, this change needs an upstream-targeted PR (retarget/recreate as appropriate; the current fork PR cannot directly change its base repository). No changes to PR Zoo-Code-Org#1622 or Zoo-Code-Org#1628 and no workspace-scopes refactoring.

Verification

  • Observed the regression test fail before the production change: the task-root manager received zero search calls.
  • After the one-line fix: focused regression and registry suites passed (15 tests).
  • Relevant code-index suite plus regression tests: 27 files, 619 tests passed.
  • Changed-file ESLint with prune-suppressions and max-warnings=0 passed; suppression data unchanged.
  • Prettier completed and git diff whitespace check passed.
  • Extension TypeScript check passed.
  • Normal commit/push hooks passed repository lint and type-check tasks (11 tasks each, 10 cached).

Non-blocking environment warnings: Node v24.7.0 differs from required 22.23.1; Prettier reports an existing unknown ignore option. Full repository tests and extension-host E2E were not run.

@WebMad
WebMad force-pushed the fix/codebase-search-task-workspace branch from 8c5f6f3 to 8dcc696 Compare September 13, 2026 12:36
…lidateToolUse (Zoo-Code-Org#1625)

* fix(delegation): read task-local mode in getEnvironmentDetails and validateToolUse

* test(delegation): cover custom-tool execute context and state fallback
Co-authored-by: Roomote <roomote@roomote.dev>
@edelauna
edelauna force-pushed the refactor/1594-code-index-manager-registry-incremental branch from 47cea56 to 59135ca Compare September 16, 2026 01:49
WebMad and others added 3 commits September 16, 2026 02:14
* refactor(code-index): extract manager registry

* test(task): mock code index registry in task suite

* test(code-index): remove redundant context casts

* refactor(code-index): apply registry review feedback

* fix(code-index): dispose registry on deactivate; drop dead registerCommands call

* fix(coderabbit): limit neighbouring review scope creep

---------

Co-authored-by: Elliott de Launay <edelauna@gmail.com>
Co-authored-by: edelauna <54631123+edelauna@users.noreply.github.com>
…g#1649)

* fix(ci): preserve coverage cache for verifier changes

* fix(ci): isolate coverage input mutation check

* fix(ci): avoid self-mutating coverage verifier

* fix(ci): isolate coverage hash probes

* fix(ci): validate cache before publication

---------

Co-authored-by: Roomote <roomote@roomote.dev>
* fix(deepseek): enable images for current Flash models

* test(deepseek): cover vision alias defaults

---------

Co-authored-by: Roomote <roomote@roomote.dev>
Co-authored-by: edelauna <54631123+edelauna@users.noreply.github.com>
zoomote Bot and others added 3 commits September 17, 2026 01:36
…or domains containing "x.ai" (Zoo-Code-Org#1484)

* fix: _isGrokXAI false-positive substring match breaks token usage for domains containing 'x.ai'

Fixes Zoo-Code-Org#1483

The _isGrokXAI() method used urlHost.includes('x.ai') which matches any
domain containing 'x.ai' as a substring (e.g. box.ai, fox.ai, max.ai).
This false-positive causes stream_options:{include_usage:true} to be
omitted, so the API never returns usage data and the token bar shows 0.

Fix: Use exact host match (api.x.ai) or subdomain match (*.x.ai) instead
of substring includes.

Added tests for false-positive scenarios and valid x.ai domain detection.

AI-assisted: developed with Zoo Code/GLM-5.2, reviewed and verified by the contributor.

* Address CodeRabbit review: use URL.hostname, bracket notation, remove changeset

* test: add O3+Grok stream_options coverage for handleO3FamilyMessage

---------

Co-authored-by: Elliott de Launay <edelauna@gmail.com>
…-Code-Org#1505)

* initial fix for issues Zoo-Code-Org#1240 and Zoo-Code-Org#505

* 1st round of fixes

* fixed comments

* increase test coverage

* revert: remove Windows shell invocation from stryker-diff

* fix: address CodeRabbit review on tool-policy prompt unification

* fix(test): correct apiModelId in generateSystemPrompt state mock

* drop use_mcp_tool from policy when no MCP tool is permitted

* code hardening

* bound model fetch with timeout, typed provider state test doubles

* cover preview model fetch timeout path with tests

* pin completion-time history save ordering with unit tests

* poll history length in restart e2e to tolerate atomic write window

* share one model-info snapshot per request between prompt and tools

* resolve provider state once before the MCP wait

getSystemPrompt read provider state twice: once for the MCP gate and
again after the hub wait. When the caller threaded no state, the two
reads could observe different snapshots. Hoist the fallback resolution
to the top of the call so the prompt and the tool guidance share one
snapshot on the unthreaded path.

Type the test harness getSystemPrompt signature with ProviderState and
ModelInfo instead of unknown, and align the affected test title and
comments with the single-read behavior.

* cover the undefined provider state path in the system prompt tests

The provider state read can resolve to nothing even while the provider
reference stays alive. Add a test for that case so the system prompt
call keeps receiving undefined disabledTools instead of failing.

* reuse one model-info snapshot per request and honor cancellation

Request construction re-read model metadata twice after the streaming
turn's bounded fetch; thread the captured snapshot through
attemptApiRequest so prompt assembly, context sizing, and tool arrays
agree on a single view, resolving the fallback only when no snapshot
was supplied.

A cancellation that lands during a request's waits now stops the
request before any tool array, abort controller, or provider call is
issued for it.

* pin the retry count the request seam receives

The empty-response retry test now asserts that the retry iteration
reaches attemptApiRequest with its own incremented attempt count
(second call, retryAttempt 1), instead of only checking the resulting
conversation history.

* refactor(task): require callers to thread provider state into system prompt build

getSystemPrompt no longer falls back to re-reading provider state; the
provider-state snapshot parameter is now required. An explicit undefined
declares that the caller's own read came back empty because the provider was
already gone, and the prompt then resolves from defaults. The prompt and the
request's runtime tool array now resolve from a single snapshot by
construction rather than by caller convention.

Behavior is unchanged on all reachable paths. Task.spec.ts grows from 128 to
129 tests to cover the required-parameter contract.

* fix(api): cancel abandoned model-metadata waits via AbortSignal

The bounded metadata waits in Task.safeEnsureModelFetched and the system
prompt preview cleared their timer but left the handler-side promise
waiting on the model-catalog fetch. The ApiHandler contract now threads
an optional AbortSignal through ensureModelFetched(): RouterProvider
settles the waiter with a rejection when the signal aborts, so an
abandoned or cancelled caller detaches instead of parking a promise on
the shared fetch (which keeps running for other waiters and still
populates the cache, by design). The task aborts its waiter both when
the 5s bound expires and when cancelCurrentRequest runs (cancel and
dispose paths); the preview aborts at its bound and on completion.

The task-lifecycle doc's table padding was also reconciled with the
PR base: the remaining diff there is now only prettier's column
re-padding, which the repo's own pre-commit formatter enforces.

* test(api): cover abort-signal detach paths and thread request model snapshot

Mutation-diff gate kills (PR Zoo-Code-Org#1505):
- zoo-gateway: signal-aware ensureModelFetched tests for the fetch-wins
  and fetch-rejects branches (block/CallExpression NoCoverage), an
  addEventListener spy pinning the { once: true } options, and paired
  add/remove listener assertions pinning the abort event name on both
  detach sites (StringLiteral mutants).
- Task: ownership-guard tests for metadataFetchAbortController (clear on
  own completion, leave a replaced controller in place).
- generateSystemPrompt: signal-capture tests pinning the timeout-bound
  and finally-block controller.abort() detaches (CallExpression mutants).

CodeRabbit: thread the request model-info snapshot into
buildCleanConversationHistory so preserveReasoning resolves from the same
per-request snapshot as the prompt and tool arrays, plus regression tests.

No Stryker-disable directives were needed; all 14 mutants are killed
behaviorally.

* Apply disabled and excluded tool policy to dynamic MCP declarations

Gate dynamic MCP tool declarations through the shared effective-tool-policy predicate (alias-resolved disabled/excluded settings). Add filter-layer and builder-layer tests covering disabled, enabled, alias, and Gemini allowlist cases. Addresses maintainer review feedback.

* Forward request options through API retry recursion

Recursive attemptApiRequest retries dropped the options argument, losing caller-provided model info on retried attempts. Forward it at all three retry sites with regression tests.

* Forward derived model snapshot through API retry recursion

when the caller omitted requestModelInfo, each retry hop re-derived the model snapshot; the first hop's snapshot is now threaded into the recursive calls (caller-supplied values keep reference identity, no caller mutation), with a regression test pinning single derivation and snapshot arrival.

* Tighten build-tools test assertions and provider double

assert the MCP tool name is retained in Gemini-declared tool lists; replace double type assertions in the provider test double with a precisely-typed local shape.

* Use the request model snapshot for context-window recovery math

After a context-window overflow the recovery handler re-fetched model metadata, so truncation could run against a newer snapshot than the retry it feeds — history could be over-truncated. The pinned request snapshot is now passed into the handler and the stale re-fetch removed, with a regression test pinning one derivation per request.

* Stop manual condensation when the task is cancelled

condenseContext awaited the best-effort model metadata fetch and then
continued even when the task had already been cancelled or abandoned, so
a summarization request could still be issued for a task that was going
away. Check for cancellation after the fetch and return early.

Add regression tests for the cancelled and abandoned cases.

* Recheck cancellation before summarizing and rewriting history

condenseContext could still issue a summarization request, and rewrite
the persisted conversation history, when the task was cancelled while the
system prompt was being built or while summarization was in flight. Check
for cancellation after each of those awaits and return early.

Add regression tests that cancel at both points and assert that neither
summarizeConversation nor overwriteApiConversationHistory runs.

* Make the first cancellation checkpoint observable to tests

The second cancellation check in condenseContext also skips summarization, so
falsifying the first one left every test passing. The mutation gate caught
this: two mutants on the first check survived because nothing observed the
work between the two checks.

Assert that a task cancelled at the first checkpoint never builds the system
prompt, which is the behavior that check exists to guarantee.

* Correct a rationale comment in the cancellation tests

The comment claimed that skipping summarization is also achieved by the
checks placed after the prompt and summarize awaits. Only the check after
the prompt await can hide a missing first check: the later one runs once
summarization has already been called.

* Narrow the change set to the tool-policy work and its regression tests

Remove the task-lifecycle and history-persistence work from this
branch: the metadata-fetch timeout bound, the waiter-detach signal
plumbing, and the post-summarization cancellation guard revert to
main; that work is preserved outside the branch for a follow-up.

What remains is the prompt/tool-policy change for Zoo-Code-Org#1240 and Zoo-Code-Org#505,
plus two fixes the review asked for. A new builder-layer test pins
that modelInfo.excludedTools excluding use_mcp_tool removes the
dynamic mcp--* declarations from the sent tools, like a user-level
disable. And a disabled or excluded attempt_completion now honors
the tool allowlist end to end: it leaves the effective policy set
and the callable allowlist, and execution rejects the call with the
standard validation-error tool_result instead of completing the
task.

* Remove dead export, untriggerable timer guard, and duplicated prompt-spec coverage

Unexport hasAnyMcpResources (no external callers), make the skills section policy parameter required (the sole caller always passes one), and make the model-metadata timeout clear unconditional (the handle is always assigned). Inline the single-use SystemPromptRequest alias and drop stale comment narration. Delete prompt-spec tests that duplicated sections.spec coverage, moving the two assertions that carried unique mutation kills (empty edit-restriction description branch, terminal-output fallback tail) into the surviving sections.spec tests.

* fix(prompts): enforce effective tool policy guidance

* fix(task): restore caller-layer cancellation for model-metadata fetches

Model-metadata fetches (ensureModelFetched) could outlive the request that
started them: a canceled task or a timed-out prompt preview left the fetch
awaited, with no signal to abort it and no check before its result was
persisted. This restores cancellation handling at the caller layer:

- The bounded preview timeout now aborts the metadata fetch it races,
  instead of leaving the fetcher's promise dangling after the timeout.
- Condense paths now check abort/abandoned state before starting and
  before persisting summarized history, with an added guard before
  summarization so a canceled task cannot write summarize output.
- cancelCurrentRequest aborts the in-flight metadata fetch and detaches
  waiters, so stale promises no longer retain task state.
- Adds a standalone edit-tool coverage test for prompt-section rendering
  (coverage gap: the tool was only exercised via combined fixtures).

Related to Zoo-Code-Org#505, Zoo-Code-Org#1240.

* fix(task): set disposal state before cancelling metadata waits

Task disposal now marks the task as aborted before it cancels the
prompts that in-flight metadata fetches are waiting on. Marking the
disposal synchronously means any model request that could start after
cleanup begins already observes an aborted task, so no request starts
after disposal.

Adds a regression test for disposal racing a metadata wait, and an
assertion that getModels is not called when the signal is already
aborted.

* chore(ci): bump coverage-contract baseline for branch-added policy module

Coverage source population moved from 469 records / 30229 lines to 470 records / 30324 lines. The delta is attributable to src/core/prompts/tools/effective-tool-policy.ts, a production module added by this change; the remaining line growth comes from branch modifications to existing instrumented sources. No source files were removed; verified by regenerating all coverage lanes locally.

* test: mock CodeIndexManagerRegistry in build-tools.spec (upstream Zoo-Code-Org#1622 merge parity)

* fix: describe codebase_search as semantic search; anchor read_file in build-tools allowlist test

Address CodeRabbit review findings on the capabilities prompt and the
build-tools test suite:

- The codebase_search capability clause said "view source code
  definitions", wording inherited from the removed
  list_code_definition_names tool; it now reads "semantically search
  the codebase", matching the tool contract, and the
  generateSystemPrompt.spec.ts assertions quoting the old phrase are
  re-pointed.
- The disabled-tools test asserted only tool absence, so an empty
  allowlist would pass; it now anchors on read_file being present,
  mirroring the sibling test.

---------

Co-authored-by: Roomote <roomote@roomote.dev>
@WebMad
WebMad force-pushed the fix/codebase-search-task-workspace branch 2 times, most recently from 01a892c to 9b8b9f4 Compare September 18, 2026 20:09
zoomote Bot and others added 10 commits September 18, 2026 22:31
…author pushes (Zoo-Code-Org#1672)

* fix(ci): preserve awaiting-author until maintainer re-review (Zoo-Code-Org#1671)

* perf(ci): memoize collaborator permission lookups in review-state workflow

---------

Co-authored-by: roomote[bot] <roomote[bot]@users.noreply.github.com>
…g#1630)

* test(code-index): readiness coverage

* test(code-index): clarify search readiness scenarios

* test(tools): cover codebase search mode permissions directly

* test(build-tools-readiness): add liveness check and extract callable helper

---------

Co-authored-by: Elliott de Launay <edelauna@gmail.com>
…oo-Code-Org#1673)

* fix(terminal): prevent inline terminal cmd.exe fallback on Windows

BaseTerminalProcess.execaOptions previously passed shell: BaseTerminal.getExecaShellPath() || true, so an unset execaShellPath fell back to shell:true. On that branch the shell process becomes a bare cmd.exe instead of the resolved PowerShell/Zoo profile, causing the inline terminal to silently downgrade to Windows Command Prompt.

Change the fallback to ?? getShell(), which resolves through VS Code profile config -> Zoo override -> userInfo -> env -> allowlisted default (never shell:true). Explicit execaShellPath still wins verbatim, and a deliberately selected cmd.exe profile is preserved via getShell(). Adds a cross-path regression suite covering explicit-win, unset->getShell(), PowerShell-via-configured-profiles, deliberate-Command-Prompt preservation, and never-shell:true.

* Fix empty execa shell path fallback

Use a truthy fallback so an empty persisted execaShellPath resolves through getShell() instead of being passed through as an empty shell value.

* Tighten shell path assertion

Assert the exact mocked PowerShell path instead of matching only the executable name.
…oo-Code-Org#1626)

* test(lifecycle): formalize remaining issue protocols

* docs(lifecycle): qualify delegated mode coverage

* docs(lifecycle): classify verification coverage

* docs(lifecycle): separate open and historical issues

* fix(delegation): read task-local mode in getEnvironmentDetails and validateToolUse

* test(delegation): cover custom-tool execute context and state fallback

* Revert "test(delegation): cover custom-tool execute context and state fallback"

This reverts commit 4909065.

* Revert "fix(delegation): read task-local mode in getEnvironmentDetails and validateToolUse"

This reverts commit c83d504.

* docs(lifecycle): make gap audit self-contained

* docs(lifecycle): add exhaustive verification gap report

* docs(lifecycle): audit subtask todo isolation

* docs(lifecycle): generalize tool state ownership gaps

* docs(lifecycle): add remediation portfolio sizing

* docs(lifecycle): separate fan-out from baseline

* docs(lifecycle): remove remediation time estimates

* docs(lifecycle): split remediation into one-point blocks

* docs(lifecycle): fix remediation block ordering

* docs: correct lifecycle gap references

* fix(lifecycle): require exact delegated mode witness

* fix(lifecycle): give fan-out invariants teeth and correct GAP-report inventory

---------

Co-authored-by: Roomote <roomote@roomote.dev>
Co-authored-by: Elliott de Launay <edelauna@gmail.com>
Co-authored-by: edelauna <54631123+edelauna@users.noreply.github.com>
Co-authored-by: Elliott de Launay <edelaunay@wealthsimple.com>
…oo-Code-Org#1700)

Co-authored-by: zoomote[bot] <305051434+zoomote[bot]@users.noreply.github.com>
…o-Code-Org#1188)

* fix(vscode-lm): sanitize surrogates, recover leaked tool calls, and window-safe tool_result truncation

Hardens the VS Code Language Model provider (notably GitHub Copilot serving
Anthropic Claude) against three failure modes:

- Surrogate sanitization: a lone UTF-16 surrogate cannot be encoded as UTF-8,
  so the backend rejects the entire request with a 400. sanitizeSurrogates()
  replaces unpaired surrogates with U+FFFD while preserving valid pairs
  (emoji, CJK ext.), applied to string messages, tool results, and text parts.

- Leaked tool-call recovery: some backends stream a tool call as raw <invoke>
  XML instead of a structured LanguageModelToolCallPart, leaving the turn with
  no tool_use block and stalling the task in a "no tools used" retry loop.
  extractLeakedToolCalls() and trailingPartialToolMarkerLength() detect the
  markup mid-stream (including markers split across chunk boundaries) and
  replay it as a real tool call, conservatively: only for <invoke> names
  matching a tool actually offered that turn, and only when tools were offered.

- Window-safe tool_result truncation: Copilot's backend trims over-window
  requests without preserving tool_use/tool_result pairing, orphaning a
  tool_result and causing a 400 (unexpected tool_use_id).
  truncateToolResultsToFitWindow() and middleOutTruncate() shrink oversized
  tool_result payloads on our side (largest first, middle-out, pairing
  preserved) before sending.

Ported from simurg79/Roo-Code#12.

* test(vscode-lm): cover leaked tool-call salvage and tool_result truncation paths

Raises patch coverage on the new vscode-lm reliability code above the 80%% codecov/patch gate by exercising the streaming salvage state machine (marker split across chunks, multi-chunk buffering, unknown-tool passthrough, carried tail) and the tool_result truncation helpers (array-form content, surrogate-safe middle-out, guard clauses).

* fix(vscode-lm): guard leaked tool-call recovery against quoted markup

Address review feedback on the leaked-tool-call salvage path: a tool name alone was not a sufficient gate, so prose or fenced examples reproducing the invoke markup could be replayed as real calls. Adds the quoted/fenced guard plus coverage.

Also records the empirical vscode.lm probe as a project skill (probe-vscode-lm-api) with the scratch probe extension, the false-positive replay harness, representative transcripts, and the consent-gate gotcha.

* chore(knip): exclude .roo skill assets from unused-file analysis

Skill directories hold reference scripts and captured artifacts that are intentionally never imported by the build.

* fix(vscode-lm): address review feedback on leaked tool-call recovery

- dispose the probe CancellationTokenSource in a finally block

* docs(skill): drop probe transcripts from repo

Remove the ~120KB raw probe transcript corpus from the vscode-lm probe skill; keep the measured findings and their stated limits in SKILL.md.

* chore: move probe skill scripts under scripts/, drop .roo knip ignore

* fix(vscode-lm): harden quoted-markup detection and bound the salvage buffer

Loop tag stripping until stable so `<<script>>` cannot reconstruct a tag
after a single pass (CodeQL incomplete multi-character sanitization).

Track fence marker and width instead of counting ``` runs for parity, so
tilde fences and 4+ backtick fences are recognized.

Treat a quoted invoke that ends its line as quoted when an explicit
quoting cue precedes it, rather than recovering it as a live tool call.
Keying off leading prose alone was tried previously and regressed genuine
recoveries, so the cue is deliberately narrow.

Bound the salvage buffer so markup that never closes is flushed as plain
text instead of withholding the response until the stream ends.

* test(vscode-lm): assert the salvage buffer flushes mid-stream

The first version of this test only checked the flushed text's content,
which the end-of-stream drain produces even without the cap, so it passed
against the unfixed code. Assert instead that text reaches the consumer
before the stream is exhausted, which is what the bound actually changes.

* test(vscode-lm): cover fence-close branch of isInsideCodeFence

Replace the vacuous four-backtick test with a nested inner-fence case and add a closed-fence recovery test, both of which fail under the old backtick-parity counting.

* fix(vscode-lm): clamp messages budget to a positive floor

Address CodeRabbit review: a system prompt or tool schema large enough to consume the derived char budget left messagesBudgetChars non-positive, which made truncateToolResultsToFitWindow a no-op exactly when the request was most oversized. Clamp to MIN_TOOL_RESULT_CHARS and cover it with a regression test. Also reattach a misplaced doc comment and dedupe a test helper.

* fix(vscode-lm): require function_calls wrapper and sanitize tool-call args

Addresses taltas review feedback on PR Zoo-Code-Org#1188.

* chore: remove probe skill and harness from PR

* docs: drop dangling probe skill path from vscode-lm comment

* fix(vscode-lm): harden streaming tool-call recovery and token budgeting

Recover only wrapped function_calls/invoke markup leaked into text parts; bare unwrapped invoke is passed through unchanged. Add narrow top-level schema-aware parameter conversion and an approximate output-budget guard, with expanded provider unit tests.

* fix: derive mutation gate base from the PR merge commit's first parent

GitHub checks out the synthetic pull request merge commit as github.sha, but pull_request.base.sha is frozen when the event is created. Once main advances, the stale base made the changed-code mutation gate attribute unrelated upstream-only files to the pull request (3294 changed executable lines across 87 files instead of 361 across the 2 files the PR actually touches).

Resolve the base from the checked-out head's first parent when the head is a merge commit, leaving non-merge heads and the merge_group path unchanged. Head stays github.sha so selector coordinates remain aligned with the checked-out tree.

* fix(vscode-lm): admit requests against the raw context budget, not the trimming floor

The clamp to MIN_TOOL_RESULT_CHARS exists only to keep tool_result trimming productive; using it for the final admission check let a request through whenever the raw budget was positive but below the floor, sending an over-window request. Judge admission against the raw budget and cover the boundary with a regression test.

Also guarantee temp-repository cleanup in the two stryker-diff pull-request-selection tests via try/finally, and move the system-prompt surrogate sanitization test out of the leaked streaming recovery group.

* fix(vscode-lm): accept an explicit null for a nullable leaked-tool parameter

declaredParamType stripped "null" from a declared ["T","null"] union, so convertLeakedParamValue rejected a literal JSON null and failed the whole leaked block closed to text. It now reports that null is permitted and the conversion consults that flag. A non-nullable object still rejects null, and a declared string keeps the literal text "null".

Also assert the streamed text chunk in the accepted-budget test, which previously drained the stream and only checked the sendRequest call.

* fix(vscode-lm): support null-only parameter schemas in leaked tool-call recovery

Handle both structured type: "null" and array type: ["null"] forms in declaredParamType so recovery emits JSON null, while continuing to fail closed for non-null values. Adds unit coverage for both helper forms and a createMessage runtime regression test with a mocked VS Code LM host.

* refactor(vscode-lm): narrow this branch to leaked tool-call recovery

Surrogate sanitization and context-window tool_result truncation are being proposed as independent changes, so remove them here. Recovery does not depend on either: it keeps the original unsanitized system-prompt boundary and no longer references the truncation helpers. Retains the null-only parameter schema fix and the stryker-diff CI prerequisite.

* refactor(vscode-lm): drop surrogate sanitization from the transform layer

Sanitization is proposed independently, so restore src/api/transform to origin/main here. Recovery does not use it; the full provider and transform suites pass without it.

* refactor(vscode-lm): defer leaked tool-call streaming integration

Keeps the complete leaked tool-call parser and its direct tests, but removes the createMessage streaming integration and its integration tests so the changed-code mutation gate stays within its per-run mutant budget. createMessage is restored byte-for-byte to the base implementation, so the parser is present but not yet activated; a follow-up change re-enables it.

* test: local validation commit for PR1188 mutation selection

* test: corrected contracts for PR1188 mutation validation

* test(vscode-lm): behavioral coverage for leaked tool-call parsing

* test(vscode-lm): cover consecutive recovered invoke blocks in one wrapper

* refactor(vscode-lm): simplify leaked tool-call parser internals

Replaces the per-call regex factories with module-scope literals scanned via matchAll, which
iterates a private clone and so cannot strand a shared lastIndex when a parameter scan stops
early. Resolves the null-only declaration in its own branch instead of a never-satisfied table
entry, and accumulates leftover text as a single string now that every segment produced by a
recovery carried the same flag. Behavior is unchanged.

* refactor(vscode-lm): own each leaked-parse regex at its call site

Each pattern is declared where it is used instead of behind a module-scope factory. matchAll
iterates a private clone, so a scan that stops early when a parameter fails its schema cannot
strand a shared lastIndex. Also drops a nullable flag that the null-only branch already settles.
Behavior is unchanged.

* docs(vscode-lm): tighten leaked-parse rationale comments

Corrects a stale note that described the null-only union as forcing a JSON parse, which the
null-only branch now settles directly, and merges two overlapping quoting-cue comments. Also
stops reporting a nullable flag for a null-only type, where it is never read.

* fix(vscode-lm): keep the resolved nullable flag computed

A literal here is unobservable, since the null-only branch settles that case before the flag is
read; the computed value keeps the resolver honest about what the union actually declared.

* fix(vscode-lm): require a whitespace-only suffix on closing code fences

* test(vscode-lm): pin whitespace-only closing fence suffix behavior

* perf(vscode-lm): make leaked tool-call scanning linear in message length

* test(vscode-lm): cover incremental scanner fence, tag, and quoting-cue boundaries

* refactor(vscode-lm): track fence state per line to remove unobservable scanner states

* test(vscode-lm): pin fence suffix handling and cross-chunk line-start tracking

* test(vscode-lm): pin line-start tracking across a deferred newline

* refactor(vscode-lm): drop redundant scan offsets from quoting state

* docs: record string-parsing performance guidance in AGENTS.md

* Revert "docs: record string-parsing performance guidance in AGENTS.md"

This reverts commit 875b0b8.

* test: replace flaky timing-based scaling assertions with deterministic work counter

The two leaked tool-call scaling tests measured wall-clock elapsed time and asserted the 4x-input ratio stayed under 10. On shared CI runners GC pauses and contention breached that even though complexity is linear (observed 14.55 and 10.02). Count characters the parser scans instead: exact, machine-independent, and still ~16x under a reintroduced quadratic prefix re-scan.

* fix(vscode-lm): keep invoke bodies out of quoting state and fail closed on unclosed params

* fix(vscode-lm): ignore function_calls wrapper tags inside quoted code

A <function_calls> opener shown inside a code fence or inline-code span armed the wrapped-only gate, so a later bare <invoke> was replayed as a real tool call. Wrapper tags are now read in source order and only outside quoted code.

* test: correct stale-base expectation for merge-result base resolution

selectFromGit now normalizes a stale base to the merge commit's first parent, so an intervening base-branch file is no longer charged to the pull request.

* fix: track inline code span width when detecting quoted wrapper tags

Backtick parity treated an even-width code span as two toggles, so a quoted <function_calls> example armed wrapped-only recovery and a later bare invoke was replayed as a live tool call. Both quoting checks now share one CommonMark-correct helper that closes a span only on an equal-width backtick run.

* test(vscode-lm): make inline code-span closure tests mutation-sensitive

The wider-backtick-run test placed the wrapper between the opener and the wider run, so its verdict was decided on a prefix ending before that run and no closure-rule mutation could change the outcome. Move the wider run ahead of the wrapper, and add a mixed wider/narrower run case plus a closed-span arming case so the span width tracking in insideInlineSpanAt is actually exercised.

* fix(vscode-lm): fail closed on nested parameter tags and malformed backtick fences

Reject a leaked parameter value that itself contains parameter markup, and do not open a backtick fence whose info string contains a backtick (CommonMark 0.31.2). Adds a regression test pinning that an inline-code run before the wrapper is not a fence.

* fix(vscode-lm): fail closed on unparseable parameter openers in inter-match gaps

parseLeakedInvokeParams validated only matched values and the trailing suffix, so a <parameter opener the strict pattern could not parse was silently skipped and a later well-formed parameter still recovered - dispatching a tool call with an argument the model wrote silently missing. Guard the gap before each match as well.

* fix(vscode-lm): honour wrapper closers in fences and block argument rebinding

Two defects in the leaked tool-call parser introduced by this PR.

The wrapper-tag scan skipped closers as well as openers inside a code
fence, so a quoted closing wrapper tag left the wrapper armed and a later
bare invoke was recovered, breaking the bare-invoke contract. Closers are
now honoured unconditionally; only openers stay fence-gated.

Injected parameter markup in a value split an invoke into adjacent
well-formed matches, letting a later match silently rebind an earlier
argument (e.g. path safe.txt to /evil) and dispatch it. Repeated
parameter names now fail closed.

Also strengthens four suppression tests to assert verbatim leftoverText
passthrough, not just an empty call list.

* fix(vscode-lm): fail closed on recovered parameters absent from the tool schema

---------

Co-authored-by: Bertan Ari <bertanari@microsoft.com>
Co-authored-by: PR1188 Local Tester <tester@local.invalid>
Co-authored-by: Elliott de Launay <edelauna@gmail.com>
@WebMad
WebMad force-pushed the fix/codebase-search-task-workspace branch from 9b8b9f4 to 3e09b14 Compare September 20, 2026 11:30
zoomote Bot and others added 4 commits September 20, 2026 15:42
* ci: reduce Windows unit-test cold starts

* fix(ci): preserve Windows Turbo cache boundaries

* test(ci): parse workflow step assertions

* ci: reduce Windows unit-test overhead

* ci: stabilize extension test cache inputs

* ci: tighten extension lane cache validation

---------

Co-authored-by: Roomote <roomote@roomote.dev>
Co-authored-by: Elliott de Launay <edelauna@gmail.com>
…-Org#1582)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…ixes Zoo-Code-Org#1371) (Zoo-Code-Org#1380)

* fix(mcp): preserve concurrent MCP settings during initial creation (fixes Zoo-Code-Org#1371)

getMcpSettingsFilePath() created the default mcp_settings.json with a check-then-write: fileExistsAtPath() followed by an unconditional fs.writeFile of the empty stub. Two windows racing at startup both saw the file as absent, and the second blind write truncated the first window's config to the 122-byte stub.

The stub write now goes through safeWriteJson with a merge callback: the read happens under the advisory lock, and any config already on disk (written by a concurrent process after the existence check) is preserved instead of clobbered. The fast path (file exists -> no write) is unchanged, so no watcher-triggered reloads or write amplification.

Test: regression test reproduces the interleaving (existence check sees absent file, locked read sees the concurrent config) and asserts the creation write carries the concurrent config, not the stub. The safeWriteJson spec mock now honors options.merge.

* test(mcp): cover getMcpSettingsFilePath fallback branches

Codecov reported 2 patch lines (1 missing, 1 partial) in the safeWriteJson merge callback. Add the three remaining fallback cases: absent file (merge sees null), existing content without an mcpServers object, and mcpServers present but not an object - all must write the default stub. All changed lines and branches of the merge callback are now covered.

* fix(fws): mcp_settings merge array guard, spec-mock production parity, unknown-safe code read

The mcp_settings merge callback now requires a plain object (!Array.isArray), so an existing mcpServers: [] is replaced by the empty stub instead of being preserved and later rejected by McpSettingsSchema; the safeWriteJson spec mock mirrors the production merge contract (only ENOENT and SyntaxError are recoverable, any other read failure rejects before the merge callback runs, with an EACCES regression); the error.code read in the mock uses an unknown-safe type guard instead of an object cast. (CodeRabbit findings on trial Zoo-Code-Org#1413).

* chore(ci): empty commit — re-trigger CI and the CodeRabbit current-head review gate (no code change)

* test(mcp): cover concurrent settings creation with real lock

* test(mcp): cover null mcpServers and malformed-JSON locked-read paths

Address review on Zoo-Code-Org#1380: pin the getMcpSettingsFilePath merge guard's truthy check on mcpServers (typeof null === 'object' and !Array.isArray(null) both hold, so only the truthy check stops { mcpServers: null } being written back — McpSettingsSchema rejects null on the next load), and cover the safeWriteJson merge contract's SyntaxError recovery (malformed JSON in the locked read becomes existing = null and the default stub is written).

---------

Co-authored-by: Eason Liang <easonliang28@gmail.com>
Co-authored-by: Elliott de Launay <edelauna@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants